Skip to content

Conversation

@aw-andre
Copy link

Fixes a flaky test in runtimes_cache_test.cpp that gives a false failure when the filesystem reuses the inode of a stale cache entry.

Closes #6168

@aw-andre aw-andre requested a review from a team as a code owner November 14, 2025 03:13
@aw-andre aw-andre requested review from josh11b and removed request for a team November 14, 2025 03:13
Comment on lines -612 to -613
auto stale_runtimes_0_inode =
stale_runtimes[0].base_dir().Stat()->unix_inode();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this will no longer compile.

Comment on lines +645 to +648
EXPECT_FALSE(stale_runtimes_0.base_dir().Stat()->unix_inode()
== stale_runtimes_0_inode &&
stale_runtimes_0.base_dir().Stat()->mtime()
== stale_runtimes_0_mtime);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[diff] reported by reviewdog 🐶

Suggested change
EXPECT_FALSE(stale_runtimes_0.base_dir().Stat()->unix_inode()
== stale_runtimes_0_inode &&
stale_runtimes_0.base_dir().Stat()->mtime()
== stale_runtimes_0_mtime);
EXPECT_FALSE(stale_runtimes_0.base_dir().Stat()->unix_inode() ==
stale_runtimes_0_inode &&
stale_runtimes_0.base_dir().Stat()->mtime() ==
stale_runtimes_0_mtime);

@chandlerc
Copy link
Contributor

I don't think this is quite the right way to fix the test. Rather than relying on timestamps, we can force the inode to not be re-used. (I hope.)

I've sent #6387 which does this, and also makes some other improvements to make it easier to debug if this keeps happening.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants